Use MSG_EOR to mark fragment boundaries#444
Conversation
2f49fb2 to
fb26b6c
Compare
|
Investigating the failures here has lead me to discover that Linux using the dgram logic does mean that it mostly behaves as-if I have also edited the initial pull request message to reflect the current state of the code, as afaiu it will become the final commit message. |
| // read, and afterwards whether it already got the entire message, | ||
| // or needs to receive additional fragments -- and if so, how much. | ||
| iovec { | ||
| iov_base: &fragment_size as *const _ as *mut c_void, |
There was a problem hiding this comment.
We should be able to use &mut here and skip the intermediate const pointer cast, but we can do that for both iovecs in a followup.
There was a problem hiding this comment.
Do you mean &raw mut fragment_size as *mut c_void? I can't seem to get the compiler to be happy with a cast directly from a reference to a void pointer.
There was a problem hiding this comment.
Well we probably still need a *mut _ cast in between, but the status quo of casting an immutable reference to a mutable pointer is obviously wrong.
ipc-channel relies on the fragments being read as atomic chunks, which is not guaranteed to be true for plain recvmsg(2) on SOCK_SEQPACKET cross-platform. Namely, on FreeBSD it's possible for this to return both more and less than a single fragment. SOCK_SEQPACKET can be thought of as a variant of SOCK_STREAM with the ability to set "end of record" markers using MSG_EOR. A single recvmsg(2) will not read past such a marker. While the fact that the marker has been reached is supposed to be communicated back to the caller through msg_flags, as of now Linux does not properly implement this on Unix-domain sockets. To handle cases where the recvmsg(2) returns less than a single fragment, include the size of the fragment in the header and continue reads until the entire fragment has been read. Since SOCK_SEQPACKET maintains ordering, as long as sending the fragments is atomic, this will not will not run the danger of interleaving of different messages. Empirically, sendmsg(2) as it is used by ipc-channel is atomic at least on FreeBSD. To make any possible resultant bugs easier to find, add asserts for that. Signed-off-by: Juhani Krekelä <juhani@krekelä.fi>
|
Applied the |
ipc-channel relies on the fragments being read as atomic chunks, which is not guaranteed to be true for plain recvmsg(2) on SOCK_SEQPACKET cross-platform. Namely, on FreeBSD it's possible for this to return both more and less than a single fragment.
SOCK_SEQPACKET can be thought of as a variant of SOCK_STREAM with the ability to set "end of record" markers using MSG_EOR. A single recvmsg(2) will not read past such a marker. While the fact that the marker has been reached is supposed to be communicated back to the caller through msg_flags, as of now Linux does not properly implement this on Unix-domain sockets.
To handle cases where the recvmsg(2) returns less than a single fragment, include the size of the fragment in the header and continue reads until the entire fragment has been read. Since SOCK_SEQPACKET maintains ordering, as long as sending the fragments is atomic, this will not will not run the danger of interleaving of different messages.
Empirically, sendmsg(2) as it is used by ipc-channel is atomic at least on FreeBSD. To make any possible resultant bugs easier to find, add asserts for that.